fix: UriTemplate.match() handles optional, out-of-order, and encoded query parameters#1785
fix: UriTemplate.match() handles optional, out-of-order, and encoded query parameters#1785felixweinberger wants to merge 10 commits intomainfrom
Conversation
Absent query parameters are now omitted from the result object rather
than set to empty string, so callers can use `vars.param ?? default`.
This also distinguishes 'param absent' from 'param present but empty'
(e.g. ?q= returns {q: ''}).
Also removes dead code: the isQuery field was always false since query
parts go to a separate array, and the (?:\\?.*)?$ regex suffix was
unreachable since pathPart already excludes the query string.
🦋 Changeset detectedLatest commit: fb65293 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
@modelcontextprotocol/client
@modelcontextprotocol/server
@modelcontextprotocol/express
@modelcontextprotocol/hono
@modelcontextprotocol/node
commit: |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| if (hasLiteralQuery) { | ||
| // Match the path regex against the full URI without a trailing | ||
| // anchor, then treat everything after the match as the remaining | ||
| // query string to parse for {?...}/{&...} expressions. | ||
| UriTemplate.validateLength(pattern, MAX_REGEX_LENGTH, 'Generated regex pattern'); | ||
| const regex = new RegExp(pattern); | ||
| match = uri.match(regex); | ||
| if (!match) return null; | ||
| queryPart = uri.slice(match[0].length).replace(/^&/, ''); |
There was a problem hiding this comment.
🔴 In the hasLiteralQuery branch (line 296-304), the regex has no trailing anchor, so it prefix-matches literal query values. For example, template /path?id=1{&extra} matched against /path?id=100 returns {} instead of null, because ^/path\?id=1 matches the prefix /path?id=1 within /path?id=100. Adding a boundary assertion like (?=[&#]|$) after the pattern would fix this.
Extended reasoning...
What the bug is
In the hasLiteralQuery branch of match() (lines 296-304), the path regex is matched against the full URI without a trailing anchor ($) or any boundary assertion. The regex pattern starts with ^ but nothing constrains where it must end. This means any literal query value in the template can prefix-match a longer value in the URI, producing false positive matches.
Concrete example: value prefix collision
Template: /path?id=1{&extra}
URI: /path?id=100
- The template has a literal
?in the string segment"/path?id=1", sohasLiteralQueryistrue. - The path regex is built as
^/path\?id=1(no trailing anchor). - Matching against
/path?id=100: the regex^/path\?id=1matches the prefix/path?id=1within the URI. - The remainder is
uri.slice(match[0].length)="00". "00"is split by&, yielding["00"]. The pair"00"has no=sign (equalIndex === -1), so it is skipped.- No query parameters are found for
extra, so the result is{}. - Expected:
null— the URI/path?id=100should not match becauseid=100≠id=1.
Second example: malformed URI accepted
Template: /path?a=1{&b}
URI: /path?a=1b=2 (no & separator — malformed)
- Regex
^/path\?a=1prefix-matches/path?a=1within/path?a=1b=2. - Remainder is
"b=2", which is split by&yielding["b=2"]. - The pair
b=2hasequalIndex=1, sokey="b",value="2". bmatches the template query part, so result is{b: "2"}.- Expected:
null— the URI is malformed (no&beforeb) and the actual value ofais1b=2, not1.
Why existing code does not prevent it
The non-hasLiteralQuery branch correctly anchors the path regex with $ and splits the URI at ?, so it does not have this problem. But the hasLiteralQuery branch intentionally omits the anchor to allow the regex to match the path+literal-query portion and then treat the rest as dynamic query parameters. The issue is that without any boundary assertion, the regex can match at any point within a query value, not just at a parameter boundary.
Impact
Any template that uses a literal ? followed by {&...} continuation operators (e.g., /path?static=1{&dynamic}) will incorrectly match URIs where the literal query value is a prefix of the actual value. This can cause incorrect resource routing — a request for /path?id=100 would incorrectly match a template expecting id=1.
How to fix
Add a lookahead assertion after the pattern to ensure the match ends at a query parameter boundary:
if (hasLiteralQuery) {
pattern += '(?=[&#]|$)'; // ensure match ends at & separator or end of string
UriTemplate.validateLength(pattern, MAX_REGEX_LENGTH, 'Generated regex pattern');
const regex = new RegExp(pattern);
// ...
}This ensures the regex only matches when the literal portion is followed by &, #, or end-of-string — not in the middle of a query parameter value.
…delcontextprotocol/typescript-sdk into fweinberger/uri-template-query-params
2 participants
Supersedes #1396. Implementation by @mgyarmathy, with a refinement to how absent query parameters are represented.
Fixes
UriTemplate.match()to correctly handle query parameter templates per RFC 6570 semantics:Motivation and Context
Fixes #149
Fixes #1079
Previously, a resource template like
products{?page,limit}would fail to matchproductsorproducts?limit=10&page=1(wrong order), returningnulland causing a "Resource not found" error. This made query parameter templates effectively unusable.Design: absent parameters are omitted, not empty string
Absent query parameters are omitted from the result rather than set to
''. This differs from #1396, which returned empty strings. Rationale:Round-trip fidelity. The SDK's
expand()already distinguishes these cases:Omitting keys preserves
expand(match(uri)) === uri. Returning''would turn/itemsinto/items?page=&limit=on round-trip.RFC 6570 §2.3 explicitly distinguishes undefined from empty string ("A variable value that is a string of length zero is not considered undefined"). The RFC only specifies expansion, not matching, but this semantic distinction should be preserved in both directions.
Ecosystem convention. The reference JS implementation (uri-templates) omits keys for absent params.
Ergonomics. Enables the idiomatic
vars.page ?? defaultValuepattern. Empty string would silently break??defaulting.This means callers can distinguish "parameter absent" (
vars.page === undefined) from "parameter present but empty" (vars.page === '', e.g.?page=).How Has This Been Tested?
New test cases cover: partial matches, out-of-order params, omitted params, extra params, encoded values, and the absent-vs-empty distinction. All 461 core tests pass.
Breaking Changes
Resource templates with query parameters now match more liberally. If you relied on strict all-params-required matching to reject requests, you'll need to validate explicitly in your callback. See
docs/migration.md.Types of changes
Checklist
Additional context
Core implementation (path/query splitting, Map-based query parsing, URL decoding) by @mgyarmathy in #1396. This PR adds the absent-param-omission refinement, changeset, and migration note.